Post processing example improvements #5902

ThierryBerger · 2022-09-07T07:33:43Z

Also:

use a triangle mesh + vertex shader for using clip space
use a post_processing module to ease integration within existing projects
unrelated fix a TextureFormat

ThierryBerger · 2022-09-07T07:36:26Z

Coming from annieversary#1, where the resizing is working 🎉

Unfortunately, at the current state of this PR, the resizing doesn't work as expected, but I'm unsure why.

I'm gonna work through differences betweens these similar looking codes and try to fix it. If anyone has ideas you're most welcome to share 👍

ThierryBerger

Resizing is working 🎉

post_processing_resizing.mp4

addressed issues

I need to research a bit how to limit the updates to the post processing material

This was adressed by bd87a21#diff-d7696ffdd45f974d95c03442261f195c34b196b587b29db22ad1ecd6eae7f4acR149

also, there is a column artifact on the side of the screen which you can see in following gif.

This is actually a shader specific problem, to have chromatic aberration working on the sides, we should render to a larger rendertarget image and/or have a bigger fullscreen triangle. I don't want to address this issue in this PR, it was already existing previously.
Another solution is to change the shader example to something simpler like a "black and white" or blur effect 🤷.

examples/shader/post_processing.rs

ThierryBerger · 2022-09-07T13:14:26Z

examples/shader/post_processing.rs

+                .spawn_bundle(Camera2dBundle {
+                    camera: Camera {
+                        // renders after the first main camera which has default value: 0.
+                        priority: camera.priority + 10,


I changed that from priority: 1 because it makes it easier to make it work with split screen example

ThierryBerger · 2022-09-07T13:20:25Z

examples/shader/post_processing.rs

+            let post_processing_pass_layer =
+                RenderLayers::layer((RenderLayers::TOTAL_LAYERS - 1) as u8);
+            let half_extents = Vec2::new(size.width as f32 / 2f32, size.height as f32 / 2f32);
+            let mut triangle_mesh = Mesh::new(PrimitiveTopology::TriangleList);
+            // NOTE: positions are actually not used because the vertex shader maps UV and clip space.
+            triangle_mesh.insert_attribute(
+                Mesh::ATTRIBUTE_POSITION,
+                vec![
+                    [-half_extents.x, -half_extents.y, 0.0],
+                    [half_extents.x * 3f32, -half_extents.y, 0.0],
+                    [-half_extents.x, half_extents.y * 3f32, 0.0],
+                ],
+            );
+            triangle_mesh.set_indices(Some(Indices::U32(vec![0, 1, 2])));
+            triangle_mesh.insert_attribute(
+                Mesh::ATTRIBUTE_NORMAL,
+                vec![[0.0, 0.0, 1.0], [0.0, 0.0, 1.0], [0.0, 0.0, 1.0]],
+            );
+
+            triangle_mesh.insert_attribute(
+                Mesh::ATTRIBUTE_UV_0,
+                vec![[2.0, 0.0], [0.0, 2.0], [0.0, 0.0]],
+            );
+            let triangle_handle = meshes.add(triangle_mesh);


I've written that positions are not used, but it's probably a lie, as if I change them a bit I notice it's not working properly ; I'm open to suggestions to explain what's happening here.

At least on my end (macos/m1), things seem to work properly as long as some positions are set, even

triangle_mesh.insert_attribute( Mesh::ATTRIBUTE_POSITION, vec![[0., 0., 0.], [0., 0., 0.], [0., 0., 0.]], );

ThierryBerger · 2022-09-07T13:22:46Z

examples/shader/post_processing.rs

+            commands
+                .entity(entity)
+                // add the handle to the camera so we can access it and change the percentages
+                .insert(material_handle.clone())


I'm not so fan of adding a raw handle as component but at least it's simple.

ThierryBerger · 2022-09-07T13:23:10Z

examples/shader/post_processing.rs

+            let material_handle = post_processing_materials.add(PostProcessingMaterial {
+                source_image: image_handle.clone(),
+                offset_r: Vec2::new(0.1f32, 0.1f32),
+                offset_g: Vec2::new(0.1f32, -0.1f32),
+                offset_b: Vec2::new(-0.1f32, -0.1f32),
+            });


those will get overwritten quickly in the update_material

IceSentry

Couple of naming nitpicks.

Can you explain why you couldn't use a Quad like the original example? This is example is already pretty complicated and using a custom mesh complicates it even more.

examples/shader/post_processing.rs

Co-authored-by: Charles <IceSentry@users.noreply.github.com>

ThierryBerger · 2022-09-07T19:21:59Z

Can you explain why you couldn't use a Quad like the original example?

It was a feedback from the original post processing PR: #4797 (review)

I was mostly curious to see if I could implement it correctly and have feedbacks about it.

You surely have a good point though, I can make it into a separate PR, where we could focus on benchmarking it too.

IceSentry · 2022-09-07T19:26:11Z

Right, I guess a fullscreen triangle would be better. Personally, it feels a bit out of scope for the example, but it does make sense to push people in that direction until we have first party support.

Co-authored-by: Charles <IceSentry@users.noreply.github.com>

ThierryBerger · 2022-09-08T09:47:08Z

Right, I guess a fullscreen triangle would be better. Personally, it feels a bit out of scope for the example, but it does make sense to push people in that direction until we have first party support.

It might be wiser to exclude it indeed from that PR, as when looking into #5902 ; I think it's a better way of implementing a fullscreen triangle than what I did.

But removing the shader vertex makes the resizing to fail (the quad no longer takes the full screen) ; so I'll let it as-is and wait for more feedbacks.

bevyengine/bevy#5902

darthdeus · 2022-11-04T21:23:12Z

examples/shader/post_processing.rs

+                    option_window_id = Some(*window_id);
+                    Extent3d {
+                        width: window.physical_width(),
+                        height: window.physical_height(),


This is incorrect and doesn't work with UI scaling. The correct solution is either

let scale = window.scale_factor(); // ... width: (window.physical_width() as f64 / scale) as u32, height: (window.physical_height() as f64 / scale) as u32,

or just the simpler alternative that already returns the logical dimensions

width: window.width() as u32, height: window.height() as u32,

darthdeus · 2022-11-04T21:24:13Z

examples/shader/post_processing.rs

+                        let window = windows.get(fit_to_window.window_id).expect("PostProcessingCamera is rendering to a window, but this window could not be found");
+                        Extent3d {
+                            width: window.physical_width(),
+                            height: window.physical_height(),


This is incorrect, same as noted here https://github.com/bevyengine/bevy/pull/5902/files#r1014476079

rparrett · 2023-02-15T15:29:14Z

It would be nice if a rendering SME could weigh in, but it seems like the vertex shader may be the cleanest way to support resizing properly. If that's the case, I feel like it is worth the extra complexity.

I think we could at least tuck the mesh generation code into a separate function.

rparrett · 2023-02-15T17:56:01Z

Actually, this seems to exist now: https://docs.rs/bevy/latest/bevy/core_pipeline/fullscreen_vertex_shader/index.html

Is that stuff useful here?

ThierryBerger · 2023-05-11T07:50:16Z

I'm closing this because the example was completely reworked with #8376

use vertex shader + attempt at resizing

d1d2a69

ThierryBerger force-pushed the 5315-fix-post-processing-resizing branch from 9c8a571 to a885e03 Compare September 7, 2022 07:38

ThierryBerger commented Sep 7, 2022

View reviewed changes

examples/shader/post_processing.rs Outdated Show resolved Hide resolved

ThierryBerger added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples labels Sep 7, 2022

ThierryBerger commented Sep 7, 2022

View reviewed changes

ThierryBerger marked this pull request as ready for review September 7, 2022 13:24

IceSentry reviewed Sep 7, 2022

View reviewed changes

examples/shader/post_processing.rs Outdated Show resolved Hide resolved

examples/shader/post_processing.rs Outdated Show resolved Hide resolved

examples/shader/post_processing.rs Outdated Show resolved Hide resolved

examples/shader/post_processing.rs Outdated Show resolved Hide resolved

Update examples/shader/post_processing.rs

61149e5

Co-authored-by: Charles <IceSentry@users.noreply.github.com>

ThierryBerger and others added 6 commits September 7, 2022 21:38

plugin for post processing

a885e03

continuously update material post processing to allow resizing

ad51631

fix resizing without updating material explicitly

bd87a21

fix warning

9cca1c5

fix wording

95465aa

Update examples/shader/post_processing.rs

824af1c

Co-authored-by: Charles <IceSentry@users.noreply.github.com>

ThierryBerger mentioned this pull request Sep 8, 2022

Post processing example doesn't support resizing #5315

Closed

few wrong wordings corrections

14f752c

ChristopherBiscardi added a commit to rust-adventure/bevy-examples that referenced this pull request Sep 12, 2022

Start from a direct port of bevyengine/bevy#5902

698af77

bevyengine/bevy#5902

darthdeus reviewed Nov 4, 2022

View reviewed changes

ThierryBerger closed this May 11, 2023

Uh oh!

Post processing example improvements #5902

Post processing example improvements #5902

Uh oh!

Conversation

ThierryBerger commented Sep 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThierryBerger commented Sep 7, 2022

Uh oh!

ThierryBerger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

I need to research a bit how to limit the updates to the post processing material

Uh oh!

Uh oh!

ThierryBerger Sep 7, 2022

Choose a reason for hiding this comment

Uh oh!

ThierryBerger Sep 7, 2022

Choose a reason for hiding this comment

Uh oh!

rparrett Feb 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThierryBerger Sep 7, 2022

Choose a reason for hiding this comment

Uh oh!

ThierryBerger Sep 7, 2022

Choose a reason for hiding this comment

Uh oh!

IceSentry left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ThierryBerger commented Sep 7, 2022

Uh oh!

IceSentry commented Sep 7, 2022

Uh oh!

ThierryBerger commented Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darthdeus Nov 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darthdeus Nov 4, 2022

Choose a reason for hiding this comment

Uh oh!

rparrett commented Feb 15, 2023

Uh oh!

rparrett commented Feb 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThierryBerger commented May 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ThierryBerger commented Sep 7, 2022 •

edited

Loading

ThierryBerger left a comment •

edited

Loading

rparrett Feb 15, 2023 •

edited

Loading

ThierryBerger commented Sep 8, 2022 •

edited

Loading

darthdeus Nov 4, 2022 •

edited

Loading

rparrett commented Feb 15, 2023 •

edited

Loading